Skip to content

Merge error-session capture onto publish - #45

Open
Fiona2016 wants to merge 89 commits into
publishfrom
conflict-error-session-capture
Open

Merge error-session capture onto publish#45
Fiona2016 wants to merge 89 commits into
publishfrom
conflict-error-session-capture

Conversation

@Fiona2016

Copy link
Copy Markdown
Collaborator

What this is

The release integration of the error-session capture feature (sessionOnError / sessionReplayOnError) onto publish. It merges the two stacked review branches (feat/error-session-replay-sampling #24 and feat/error-session-event-sampling #25) and resolves the conflict with the "a rate that leaves zero" work already on publish.

Opened to run CI on the exact tree that would ship. Not for merge yet — pending release go-ahead.

Conflict resolution

  • rumSessionManager.ts: the zero-rate emergency stop keeps its on-error guard — a rate of 0 spares only a session that is itself on-error (withholdsEvents), and still ends a plainly drawn one. The not-tracked branch also ends a session when the switch turns on at rate 0, matching the existing rate-above-0 behaviour.
  • CHANGELOG.md: the Unreleased block sits above v0.2.3.

Verification

yarn test:unit (3170 passed, 0 failed), yarn typecheck, yarn lint, yarn format all clean on the merged tree.

🤖 Generated with Claude Code

…export from

mockProfiler imported registerCleanupTask and getGlobalObject from the rum
package rather than core, and profiler.spec.ts imported from package names this
repository does not publish. Since mockProfiler is re-exported from the rum test
barrel, the broken imports took every spec that touches that barrel down with
them - around 220 tests never ran.
A session drawn by this rate records from the start but uploads nothing until it
reports an error. If none ever happens, nothing is sent and the session is never
stored. On the first error the withheld buffer is released and recording
continues normally, so the replay covers what led up to the error rather than
starting at it.

The buffer is bounded on both axes. Time: a buffer that spans more than a minute
is dropped and restarted from a fresh full snapshot, so what is released stays a
minute at most. Size: the existing segment byte limit still applies while
withheld, and restarts are spaced out so that a document whose full snapshot
alone exceeds that limit degrades instead of restarting in a loop.

A withheld buffer belongs to the session that produced it. It is released only
when that same session reports the error - if the session expires or is renewed
first, the records are dropped, so an expiry can never turn into an upload for a
session that never errored. Buffers that are dropped roll back their replay
stats, and has_replay is not reported while a replay is being withheld, so
neither the counters nor the link offer a replay that does not exist.

Errors raised by the SDK about its own transport do not release anything: those
are our failures, not the application's, and counting them would make every
session an error session wherever our endpoint is unreachable.
A session drawn by this rate collects events but uploads nothing until it reports
an error. If none ever happens the session is never stored, and on the first
error the withheld history is released so the detail leading up to the error is
there rather than starting at it.

Events are held upstream of the batch, which cannot serve as the buffer itself:
ordinary events go straight into a compression stream and cannot be evicted one
by one. View events are kept one-per-view and out of the eviction budget, since
the backend builds the session row from them and a detail released without its
view would be unreachable - anything whose view is gone is dropped at release for
the same reason.

The buffer is bounded by time, count and size. When it runs out of room it drops
long tasks and unremarkable requests first, then actions, and never errors. The
release is spread over a few seconds keyed on the session id, because correlated
errors would otherwise have every client release at the same instant, and it is
flushed early if the page is about to go rather than lost to that window.

The replay of such a session is withheld alongside its events, whichever replay
rate it drew: until the events are released the session does not exist yet, so a
replay sent then would have nothing to attach to and would be stranded for good
if the error never came. Forcing capture releases both, for the same reason.
The flush path derived the same thing twice under two names, and the mapping of
the internal checkout reason onto a schema value only ever had one caller.
Drops exports nothing outside the module uses, names the entry being appended
instead of reading it back off the end, folds the two ways of emptying the buffer
into one, and records why a view is deleted before being set again.
A page-exit rotation used to throw the buffer away, and with it the full snapshot
a released replay has to start from - everything recorded afterwards is
incremental and cannot be played on its own. Switching tabs raises this exit, and
the page comes straight back, so an error reported after that would have released
a replay that renders as good as nothing until the next view.

Nothing can be sent while withheld, so there was never anything to gain from the
rotation. A page that is really unloading takes the buffer with it either way.
…r was missed

Two problems with releasing a withheld event buffer.

The jitter meant to spread correlated releases did not spread them. Session ids
are same-length strings over one small alphabet, so summing their character codes
put over 97% of them within 600ms of each other: the herd was delayed by about
two and a half seconds rather than broken up. A multiplicative hash spreads them
evenly across the window, which a distribution test now pins down.

The other is that a session can report its error without the buffer noticing. The
event arrives synchronously, but the state behind it is written through a lock
that can defer the write, so the buffer may still read the session as
withholding, hold the error, and schedule nothing. If the user then leaves - which
is exactly the case this feature exists for - the whole session was thrown away.
The session is now re-read before the buffer is discarded on page exit.
… is evicted 'never'

The size budget measured UTF-16 code units, which understates non-ASCII payloads
by up to three times - a buffer meant to stay inside a beacon could be well past
it before the cap noticed.

The error tier was documented as never evicted, but the eviction loop included it
and took the oldest first: under an error storm the buffer would give up the very
first error, the one that released it and the one the session is about. Errors are
now given up only once nothing else remains, newest first.
…s go

Three lifecycle gaps in the withheld event buffer.

Nothing reacted to the session ending. A release waiting on its jitter was lost
if the session expired first, and a buffer belonging to a session that ended
because tracking consent was withdrawn stayed in memory until some later event
happened to arrive. The session ending is now settled the same way the page going
away already was.

Its stop was never wired into the SDK teardown, so a pending release could still
fire into a batch that had stopped flushing.

Views were kept for as long as the page lived, one per route, which grew past the
detail budget itself and put fifty of them into a release. A view is kept as the
container of the detail hanging from it, so it now goes once none of its detail
is left inside the window - except the view in progress, which is the container
the error will hang from.
…ased buffer

Only the rotation notices that the withheld replay has been released, so a
session that expires within one rotation of its own error still loses what the
error had earned. Closing it would mean asking the session manager on every
record.
The rollback that gives a dropped buffer's index_in_view back only lands when the
encoder finishes, which is always a turn later. Restarting from a fresh full
snapshot emitted records right away, so the next segment took its index before
the rollback arrived - and once that session errored, two uploaded segments
claimed the same index within one view while nothing claimed the first. Any error
session that spends a minute on one view before erroring hit it.

The restart now happens where the rollback lands.

Also corrects a comment: a session expiring right after its own error does not
lose the buffer. The history entry is still open when the recorder is stopped, so
the stop flush sees the session as released and sends.
…ail marker survive

Two problems the replay side had already reasoned its way out of, which the event
side had not.

The buffer was cleared on any page exit, and a page being hidden raises one -
switching tabs, or switching apps on mobile, wiped the withheld minute and left
an error arriving just afterwards with almost nothing. A page that is really
unloading takes the buffer with it anyway, so there was never anything to gain.
The session ending is different, and still clears it.

The marker saying how far back the stored detail reaches was stamped on the view
events being released, but the batch upserts views by id: the next ordinary view
update, seconds later and without the marker, replaced them before the batch was
ever sent. For the view the error happened in - the one that matters - it never
arrived. It is now recorded on the session, so every later view update carries
it.
Without it, a replay collected under this rate is indistinguishable from one
collected unconditionally once it has been uploaded - the two cost differently
and answer different questions, and nothing downstream could tell them apart.
Marking a session as having reported an error is only useful to a session that is
withholding its replay. Doing it for every session wrote the session store for customers
who enabled no error sampling at all, and that write also pushes the session's expiry out,
which moves where their sessions end.
…take it

Flushing a segment always waits for a round trip to the deflate worker, because the
trailer is written just before finishing. The collection state is reset synchronously, so
a record arriving during that round trip created the next segment while the dropped one
was still counted: two uploaded segments then claimed the same index_in_view, and index 0
was never uploaded at all.

Each counter is now given back in the phase it was taken in - the segment count
synchronously, the record and byte counts in the flush callback.
… current one

Assembly resolves a session at the event's own start time, so a request or a view update
that finishes after its session ended still carries that session's id. The buffer read
whichever session was current instead, which let two things through: a straggler of a
session that had ended without ever reporting an error was uploaded on its own - storing
the very session the withholding was there to avoid - and one arriving after a renewal was
held in the new session's buffer and released by an error that was not its own.

A view that already ended no longer becomes the current view when it is updated late
either. It carries its own start date, and treating it as current had the pruning drop the
view the next error hangs from, so the release filtered that error out of its own buffer.
Two gaps left by withholding events as well as replays. The mark that releases a buffer was
being skipped for a session that withholds only its events, since the check knew about the
replay side alone. And a buffer whose session had been renewed into one that withholds
nothing was left behind until the session expiry notification arrived, rather than being
dropped as soon as the session it belonged to was no longer current.
…d on

A store write goes through a lock and can be retried for up to a second, and other tabs
write the same store meanwhile - so the state a write lands on is not necessarily the one
it was decided against. Updates are now expressed as a function of that state, and
returning nothing makes the write a no-op, which is what a caller needs to say "only if
this is still the session I meant".
Marking a session as having errored merged into whatever session the store held at the
moment the write went through. A session that rolled over while the write waited for the
lock - or that another tab renewed - was marked instead, and then uploaded a whole session
that never reported anything. The mark now names the session it belongs to and is dropped
if that session is gone.

The same mark is also applied to the in-memory session straight away rather than only once
the write lands, because until then the withheld buffer still reads the session as
withholding: an error followed closely by the page or the session ending threw away the
very buffer the error was meant to release.
Dropping a withheld buffer restarts it from a fresh full snapshot, and that runs in a flush
callback which only arrives after a round trip to the deflate worker. Recording stopped in
between still got a full re-serialization of the document, and its records counted into the
replay stats with no segment to hold them.
…sion-event-sampling

# Conflicts:
#	packages/rum-core/src/domain/rumSessionManager.ts
…an one discarded session

Three things a withheld event buffer got wrong once time or tabs were involved.

The window it releases was measured from the moment the release ran rather than the moment
it was scheduled. The timer carrying a release is clamped to roughly once a minute in a
backgrounded tab, so by the time it ran the whole minute before the error had aged out -
the release delivered the error and nothing leading up to it. The window is now fixed when
the release is scheduled.

Only the last thrown-away session was remembered, so a request that outlived two withheld
sessions was uploaded on its own when it finally completed. A handful are remembered now,
which is more than can still be assembled to.

Where the stored detail starts is now the earliest point any tab reached, decided under the
store lock, instead of whichever tab wrote last; and it is only recorded on the session it
was measured for.

Also records what the ordering between the page-exit relay and the batch is for, since
nothing but the order of two statements enforces it.
A replay kept only because the session errored answers "do I want to see
this error's session" - and that is a yes or a no, not a share. Keeping a
random half of the error replays would just leave half the reports
uninvestigable, and the cost this could guard against is already bounded
by sessionReplaySampleRate and by turning the option off.

The rate also hid an arithmetic trap: it applied to whatever the plain
rate missed, so the real share was (100 - sessionReplaySampleRate) * rate
/ 100, and a rate set next to a plain rate of 100 silently did nothing.
A switch has nothing to multiply.

`sessionReplayOnErrorSampleRate: number` becomes
`sessionReplayOnError: boolean`, default false. The tracking types and
the session cookie are unchanged: what was drawn is now simply applied.
Merge branch 'feat/error-session-replay-sampling', which turned
sessionReplayOnErrorSampleRate into the sessionReplayOnError switch, and
give sessionOnErrorSampleRate the same treatment for the same reasons.

Whether a session that errored should be kept is a yes or a no: each such
session is wanted for its own sake, so keeping a random share of them
only leaves the rest of the error reports uninvestigable. The volume this
adds is already bounded by sessionSampleRate and by switching it off.

The rate also hid an arithmetic trap - it applied to what the plain rate
missed, so next to the default sessionSampleRate of 100 it silently did
nothing. A switch has nothing to multiply; the warning for that
combination stays.

`sessionOnErrorSampleRate: number` becomes `sessionOnError: boolean`,
default false. The tracking types and the session cookie are unchanged.
The error-session replay work was cut from main, but the fork integrates on
publish, which has since gained remote configuration, forced sessions, direct
replay upload and page reactivation. Resolved so that:

- the on-error replay switch sits inside the draw branch, after the remote
  and beforeSampling-resolved replay rate has missed, so a session is still
  never counted by both;
- forcing a session releases a withheld replay the same way it forces one
  onto a session without replay, since the host asked for it now;
- a withheld buffer survives page reactivation: the fresh full snapshot taken
  on that event lands inside the buffer, which stays replayable from it,
  where cutting would only throw away what came before the switch;
- the bridge stub never withholds, whether the host records or this page
  uploads directly.
…sion-event-sampling

Brings the publish line in underneath the on-error session work. Resolved so
that the on-error session switch sits in the draw branch after the remote and
beforeSampling-resolved rates, the bridge stub never withholds, and the stored
sessions the specs plant carry the expiry a session now has to prove.
The console can now deliver the switch beside the rates, so an operator
turns error replays on or off without shipping a release. It is read at the
draw like the rates and latched the same way: a session either withholds its
replay from the start or never does. `beforeSampling` is not offered it - a
switch is a yes or a no the console already answered.

A delivered value that is not a boolean is dropped, so it reads as "not
delivered" rather than as either position.
Merge branch 'feat/error-session-replay-sampling', which reads the replay
switch from remote configuration, and read the session switch the same way:
delivered beside the rates, latched at the draw, a non-boolean dropped as
"not delivered". `beforeSampling` is offered neither switch.
endSessionIfSettingsAreDecisive ends a running session when the settings
just delivered resolve the session sample rate to zero. With sessionOnError
a zero rate is the switch's ordinary setting rather than a stop: at a zero
rate a session is tracked exactly when the switch is on (a replay-on-error
switch cannot keep one on its own, the session draw fails first). Ending it
there discarded the very session the switch exists to keep and left the page
blind from the first configuration fetch - which lands on every fresh profile
and after every deploy - until the visitor's first interaction, so page-load
errors were never captured.

Gate the expire on the switch being off: a zero rate still stops plain
sessions, and one turned off from the console still stops on-error ones, but
a zero rate beside an on switch no longer ends the session. Add specs for
both directions.
…d branches

- startSessionReplayRecordingManually's doc still stated the pre-switch
  default rule; describe the derived default that also accounts for
  sessionReplayOnError and remoteConfigurationEnabled.
- The mark guard comment said "neither rate"; both are switches now.
- Add specs for two withheld-buffer branches that no test reached: a page
  reactivation must not cut the withheld buffer, and the SESSION_RELEASED
  subscription must wake the deferred snapshot restore with no rum event.
…ng' into feat/error-session-event-sampling

# Conflicts:
#	packages/rum-core/src/domain/configuration/configuration.ts
…ive spec gaps

- Add a changelog entry for sessionOnError / sessionReplayOnError.
- The setSessionHasError doc and the error-tracking doc said the mark only
  releases a withheld replay; it now also releases withheld events.
- Type the three fork-added session marker fields at the assembly site so a
  misspelled key fails the build instead of the schema's index signature
  quietly accepting it.
- Cover five branches no spec reached: the console turning sessionOnError off,
  tiered eviction keeping an older action over newer long tasks, an event
  collected after the release flowing through instead of being held again,
  sampled_for_replay staying off for an error-replay session that has not
  errored, and the manual-start warning staying silent when replay is off.
…oversized-error release

- An on-error session (kept by sessionOnError despite the plain draw missing
  it) now reports session_sample_rate 0 instead of the plain rate. The console
  extrapolates stored sessions by 100/session_sample_rate; reporting the rate
  that missed the session had each error session counted as 100/rate sessions.
  Reported after the draw ladder so the tracking type is known; a type that
  only withholds its replay still reports the plain rate it was drawn under.
- The never-applies / no-session-tracked init warnings no longer fire under
  remoteConfigurationEnabled, where the init rates are a fallback the console
  can override - they were false-positiving on the documented remote-config
  setup that omits the rate.
- A releasing error larger than the buffer budget now forwards on its own and
  schedules the history's release behind the jitter, instead of releasing the
  whole history in the same tick and defeating the anti-thundering-herd spread
  for the correlated outage the jitter exists for.
…nd two buffer edges

- The zero-rate emergency stop now spares only the session that is itself an
  on-error one (withholdsEvents its type), not every session while the switch
  is merely on: a plainly drawn session is still ended by a rate-0 publish and
  redraws as on-error next action, instead of continuing to upload in full.
- beforeSampling returning 0 for a rate now also clears the matching on-error
  switch, so the documented "0 never collects" is not quietly turned into
  "collect on error" for an excluded visitor.
- A session whose type an older shared-cookie bundle rewrote under the same id
  is no longer blacklisted: its withheld buffer is dropped, but its events go
  on uploading as the plain session it now is.
- A single non-error event larger than the whole buffer budget is dropped
  rather than held, so it cannot evict the minute of history to make room it
  could never fit into.
Record three known limitations of sessionOnError / sessionReplayOnError in the
changelog, and note the replay's view-boundary in the sessionReplayOnError doc:
consent-gated recording must set startSessionReplayRecordingManually explicitly;
the released replay reaches back only to the error's view while events reach back
the full minute; and under the opt-in compressIntakeRequests a tab close within
seconds of the first error can lose that release.
Resolves the two conflicts with the "a rate that leaves zero" work already on
publish: the zero-rate emergency stop keeps its on-error guard, so a rate of 0
spares only a session that is itself on-error and still ends a plainly drawn
one; the not-tracked branch also ends a session when the switch turns on at
rate 0; and the CHANGELOG Unreleased block sits above v0.2.3.
…der the same id

The same-id guard added in 9eef19d only speaks on the event path, but
the store poll always speaks first: it expires the session on the type
change, and the expiry discards the buffer with the blacklist on. The
session is then renewed under the SAME id by the store, and every event
it collects is dropped for the rest of its life - a session the backend
goes on storing and billing.

Enforce the blacklist only against a session that is not the current
one: a blacklisted id that is nonetheless live can only come from such
a foreign same-id redraw, while a session that truly ended comes back
with a new id, so real stragglers are still dropped. Cover the actual
flow (poll expiry, then same-id renewal) with a spec.
The lock field fell into the carry-unknown-fields path: every access
rewrote the cookie with the lock kept and the expiry renewed for a
year. The modern bundle has no stale-lock recovery, so a lock whose
owner was gone (a crashed tab, or one whose write we raced) stayed
alive for as long as a legacy page kept the cookie warm - wedging the
modern session store: every write retried and dropped, the in-memory
session never expiring, and every new page's init failing on an empty
cache.

Dropping the field at parse time turns the legacy rewrite into a
stale-lock cleaner, and a legacy write that lands inside the modern
lock window now fails the modern corruption check (a retry) instead
of passing it with the session silently rolled back.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant